Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (37)
🚧 Files skipped from review as they are similar to previous changes (25)
📝 WalkthroughWalkthroughAdds a WASM web runtime bridge with host-call dispatch, browser HTTP/filesystem sysops, typed handle and media APIs, runtime initialization, TypeScript bindings, declaration synchronization, and browser/worker tests. ChangesWASM web bridge
Artifact size baselines
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant BamlRuntime
participant WebBridge
participant WasmHost
participant WebHttp
participant JavaScript
BamlRuntime->>WebBridge: initialize runtime
WebBridge->>WasmHost: configure host callables
WebHttp->>WasmHost: dispatch fetch request
WasmHost->>JavaScript: invoke registered callable
JavaScript-->>WasmHost: complete host call
WasmHost-->>WebHttp: return parsed response
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
Binary size checks passed✅ 7 passed
Generated by |
sxlijin
marked this pull request as ready for review
July 20, 2026 07:43
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
baml_language/sdks/typescript/bridge_typescript_web/scripts/sync-wasm-declarations.mjs (1)
51-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
--checkmode diffs raw file bytes, which is sensitive to line-ending differences.
checkedInSource !== browserSourceis an exact string comparison of the full generated file. If the repo's checked-in file and a freshly-builtdist/wasm/bridge_web_core.d.tsdiffer only in line endings (e.g., CRLF vs LF depending on gitautocrlf/OS), this check will report false staleness. Consider normalizing line endings (or comparing the normalized export maps plus a content hash) before the exact-match check.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript_web/scripts/sync-wasm-declarations.mjs` around lines 51 - 56, Update the --check branch in the sync-wasm-declarations script to normalize line endings in both checkedInSource and browserSource before comparing them, while preserving the existing stale-file error for substantive content differences and the write behavior outside check mode.baml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-workerd-package.mjs (1)
29-37: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valuePrepended imports could collide if
workerd/index.jsalready imports the same bindings.This unconditionally prepends
import { readFileSync } from "node:fs";andimport { installReadFileSync } from "./native.js";to the generatedworkerd/index.js. If the tsc/wasm-pack-generated entry ever importsnode:fs(or re-exportsinstallReadFileSync) itself, this produces a duplicate-bindingSyntaxErrorat module load. Low risk today given the current generated output, but worth a defensive check (e.g., skip injection if the marker/import already exists) if this script runs more than once or the generated entry changes shape.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-workerd-package.mjs` around lines 29 - 37, Update the injection logic in the workerd package preparation script to detect whether the generated workerd index already contains the prepended imports or installation marker before adding them. Skip the injection when those bindings are present, while preserving the existing setup for entries that do not contain them.baml_language/sdks/typescript/bridge_typescript_web/src/media.rs (1)
35-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate handle-type validation logic vs.
handle.rs::key_with_expected_type.
media_keyre-implements the same "compareparts.handle_typeto an expected type, else returnunexpected_handle_type" logic already present inhandle.rs'skey_with_expected_type(same-crate, same file layer). Consider exposingkey_with_expected_type(e.g. viacrate::handleor a shared module) and havingmedia_keydelegate to it, e.g.key_with_expected_type(operation, parts, expected_handle_type(kind)), to keep the validation contract in one place.Based on the
handle.rssnippet shown in the graph context:fn key_with_expected_type( operation: &'static str, parts: HandleParts, expected: BamlHandleType, ) -> Result<u64, JsError> { ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/sdks/typescript/bridge_typescript_web/src/media.rs` around lines 35 - 45, Update media_key to reuse handle.rs::key_with_expected_type instead of duplicating handle-type comparison and unexpected_handle_type error construction. Expose key_with_expected_type through the crate as needed, pass operation, parts, and expected_handle_type(kind), and preserve the existing Result<u64, JsError> behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@baml_language/sdks/typescript/bridge_typescript_web/src/lib.rs`:
- Around line 10-18: Gate the errors module with #[cfg(target_arch = "wasm32")]
alongside handle, host_value, media, and runtime. Keep its existing module
declaration and ensure errors is compiled only for wasm32 builds.
---
Nitpick comments:
In
`@baml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-workerd-package.mjs`:
- Around line 29-37: Update the injection logic in the workerd package
preparation script to detect whether the generated workerd index already
contains the prepended imports or installation marker before adding them. Skip
the injection when those bindings are present, while preserving the existing
setup for entries that do not contain them.
In
`@baml_language/sdks/typescript/bridge_typescript_web/scripts/sync-wasm-declarations.mjs`:
- Around line 51-56: Update the --check branch in the sync-wasm-declarations
script to normalize line endings in both checkedInSource and browserSource
before comparing them, while preserving the existing stale-file error for
substantive content differences and the write behavior outside check mode.
In `@baml_language/sdks/typescript/bridge_typescript_web/src/media.rs`:
- Around line 35-45: Update media_key to reuse handle.rs::key_with_expected_type
instead of duplicating handle-type comparison and unexpected_handle_type error
construction. Expose key_with_expected_type through the crate as needed, pass
operation, parts, and expected_handle_type(kind), and preserve the existing
Result<u64, JsError> behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7436e7c4-8928-4b0f-9245-7b810cbc14c9
⛔ Files ignored due to path filters (2)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/sdks/typescript/bridge_typescript_web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
baml_language/Cargo.tomlbaml_language/crates/bridge_cffi/Cargo.tomlbaml_language/crates/bridge_cffi/src/handle.rsbaml_language/crates/bridge_cffi/src/lib.rsbaml_language/crates/bridge_ctypes/src/handle_table.rsbaml_language/crates/sys_wasm/Cargo.tomlbaml_language/crates/sys_wasm/src/host_value.rsbaml_language/crates/sys_wasm/src/lib.rsbaml_language/crates/sys_wasm/src/send_wrapper.rsbaml_language/crates/sys_wasm/src/web_sysops.rsbaml_language/sdks/typescript/bridge_typescript_web/Cargo.tomlbaml_language/sdks/typescript/bridge_typescript_web/package.jsonbaml_language/sdks/typescript/bridge_typescript_web/pnpm-workspace.yamlbaml_language/sdks/typescript/bridge_typescript_web/scripts/prepare-workerd-package.mjsbaml_language/sdks/typescript/bridge_typescript_web/scripts/sync-wasm-declarations.mjsbaml_language/sdks/typescript/bridge_typescript_web/src/errors.rsbaml_language/sdks/typescript/bridge_typescript_web/src/handle.rsbaml_language/sdks/typescript/bridge_typescript_web/src/host_value.rsbaml_language/sdks/typescript/bridge_typescript_web/src/lib.rsbaml_language/sdks/typescript/bridge_typescript_web/src/media.rsbaml_language/sdks/typescript/bridge_typescript_web/src/runtime.rsbaml_language/sdks/typescript/bridge_typescript_web/tests/bridge-web-core.d.tsbaml_language/sdks/typescript/bridge_typescript_web/tests/host_value_registry.test.tsbaml_language/sdks/typescript/bridge_typescript_web/tests/wasm_exports.test.tsbaml_language/sdks/typescript/bridge_typescript_web/tests/worker.jsbaml_language/sdks/typescript/bridge_typescript_web/typescript_src/native.tsbaml_language/sdks/typescript/bridge_typescript_web/typescript_src/wasm/bridge_web_core.d.tsbaml_language/sdks/typescript/bridge_typescript_web/vitest.web.config.tsbaml_language/sdks/typescript/bridge_typescript_web/vitest.workers.config.tsbaml_language/sdks/typescript/bridge_typescript_web/wrangler.test.jsonc
Comment on lines
+10
to
+18
| mod errors; | ||
| #[cfg(target_arch = "wasm32")] | ||
| #[wasm_bindgen(js_name = callFunctionSync)] | ||
| pub fn call_function_sync(function_name: &str, encoded_args: &[u8]) -> Vec<u8> { | ||
| bridge_cffi::call_function_in_wasm_sync(function_name, encoded_args) | ||
| } | ||
|
|
||
| pub mod handle; | ||
| #[cfg(target_arch = "wasm32")] | ||
| #[wasm_bindgen(js_name = callFunction)] | ||
| pub async fn call_function(function_name: &str, encoded_args: &[u8]) -> Vec<u8> { | ||
| bridge_cffi::call_function_in_wasm(function_name, encoded_args).await | ||
| } | ||
| pub mod host_value; | ||
| #[cfg(target_arch = "wasm32")] | ||
| pub mod media; | ||
| #[cfg(target_arch = "wasm32")] | ||
| pub mod runtime; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Inconsistent cfg gating on mod errors;.
handle, host_value, media, and runtime are all gated behind #[cfg(target_arch = "wasm32")], but mod errors; at Line 10 is not. Since errors.rs uses wasm_bindgen/js_sys types (JsValue, js_sys::Error) and is only consumed by the wasm32-gated modules, compiling it unconditionally means it becomes dead/unused code on any non-wasm32 build of this crate (e.g., a workspace-wide cargo check/cargo test --lib), and risks lint/dead-code noise or accidental breakage if errors.rs ever grows a non-wasm32-incompatible dependency.
🔧 Proposed fix
+#[cfg(target_arch = "wasm32")]
mod errors;
#[cfg(target_arch = "wasm32")]
pub mod handle;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| mod errors; | |
| #[cfg(target_arch = "wasm32")] | |
| #[wasm_bindgen(js_name = callFunctionSync)] | |
| pub fn call_function_sync(function_name: &str, encoded_args: &[u8]) -> Vec<u8> { | |
| bridge_cffi::call_function_in_wasm_sync(function_name, encoded_args) | |
| } | |
| pub mod handle; | |
| #[cfg(target_arch = "wasm32")] | |
| #[wasm_bindgen(js_name = callFunction)] | |
| pub async fn call_function(function_name: &str, encoded_args: &[u8]) -> Vec<u8> { | |
| bridge_cffi::call_function_in_wasm(function_name, encoded_args).await | |
| } | |
| pub mod host_value; | |
| #[cfg(target_arch = "wasm32")] | |
| pub mod media; | |
| #[cfg(target_arch = "wasm32")] | |
| pub mod runtime; | |
| #[cfg(target_arch = "wasm32")] | |
| mod errors; | |
| #[cfg(target_arch = "wasm32")] | |
| pub mod handle; | |
| #[cfg(target_arch = "wasm32")] | |
| pub mod host_value; | |
| #[cfg(target_arch = "wasm32")] | |
| pub mod media; | |
| #[cfg(target_arch = "wasm32")] | |
| pub mod runtime; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@baml_language/sdks/typescript/bridge_typescript_web/src/lib.rs` around lines
10 - 18, Gate the errors module with #[cfg(target_arch = "wasm32")] alongside
handle, host_value, media, and runtime. Keep its existing module declaration and
ensure errors is compiled only for wasm32 builds.
sxlijin
force-pushed
the
sam/typescript-web-wasm-runtime
branch
from
July 20, 2026 08:01
a1c1154 to
81a72ae
Compare
meefs
pushed a commit
to meefs/baml
that referenced
this pull request
Jul 20, 2026
…yML#4006) ## Summary - integrate the TypeScript Web bridge with the shared engine and TypeScript runtime APIs - fill out browser and Cloudflare Workers system operations and connect generated SDKs to the functional runtime - remove the Web and Workers Vitest ignore annotations once the complete SDK surface is available ## Stack This PR is based on BoundaryML#4091. The parent provides the independently buildable raw WebAssembly runtime, target-gated CFFI hooks, packaging, and focused browser/workerd tests. This PR layers the shared engine and TypeScript API changes plus full generated SDK parity on top. ## Validation - cargo nextest run -p sdk_test_typescript_web -E all() --no-fail-fast: 26 passed, 0 skipped - TypeScript Web SDK matrix passes on Linux, macOS, and Windows - mise run fmt, mise run stow, and mise run clippy pass <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Web runtime sysops for HTTP `_fetch/_send` and synchronous filesystem reads. * Expanded the Web bridge with runtime initialization, plus exported handle/media utilities and host-call lifecycle support. * Added `getVersion`/`flushEvents` and strengthened call-context lifecycle handling for Web. * **Bug Fixes** * Improved host-value/callable validation, stricter error/panic decoding, and safer cancellation/detach behavior. * Enhanced type-context conversions for arrays/maps/instance fields. * **Tests** * Expanded SDK Web test coverage (Web + Workers) and improved fixture harness generation. * **Chores** * Refreshed size-gate baselines and updated Web wasm/type declaration sync tooling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
The lower PR in the TypeScript Web stack imported sys_wasm from a later commit, so it could not build independently. This change makes the raw WebAssembly runtime a self-contained foundation while leaving shared engine and TypeScript API changes for the stacked integration PR.
Impact
Browser and Workers consumers gain a buildable raw WebAssembly runtime foundation. Native bridge_cffi behavior remains unchanged because the new CFFI and handle-table hooks are gated to wasm32.
Validation
Summary by CodeRabbit